-
Notifications
You must be signed in to change notification settings - Fork 347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Experiment and dataset improvements #6163
Conversation
7aecb06
to
6d59100
Compare
if span_kind == LLM: | ||
return _get_llm_span_input( | ||
input_messages=input_messages, | ||
input_value=input_value, | ||
input_mime_type=input_mime_type, | ||
prompt_template_variables=prompt_template_variables, | ||
tool_definitions=tool_definitions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just double check that this works with openai ft format? also it might be just simpler if the key was tools
if tool_definitions_data := [ | ||
_safely_json_decode(tool_definition) for tool_definition in tool_definitions | ||
]: | ||
input["tool_definitions"] = tool_definitions_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input["tool_definitions"] = tool_definitions_data | |
input["tools"] = tool_definitions_data |
Kinda leaning this direction. Does this work with openai ft / evals format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the tool definitions are a key alongside the input:
https://platform.openai.com/docs/guides/fine-tuning#preparing-your-dataset-for-dpo
@@ -795,7 +795,7 @@ async def chat_completion_create( | |||
elif isinstance(event, anthropic_streaming.InputJsonEvent): | |||
raise NotImplementedError | |||
else: | |||
assert_never(event) | |||
assert_never(event) # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a fix for this that @RogerHYang put out. It's the anthropic citations event. Can you cherry pick that and remove this ignore?
|
||
# ToolAttributes | ||
LLM_TOOLS = SpanAttributes.LLM_TOOLS | ||
TOOL_DEFINITION = ToolAttributes.TOOL_JSON_SCHEMA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can rename to TOOL_JSON_SCHEMA
for consistency with the name of the convention
resolves #6082
resolves #6153
resolves #6139